-
Notifications
You must be signed in to change notification settings - Fork 352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a module and class for handling sky grids for pycbc_multi_inspiral
#4741
Conversation
Note that I posed a couple design questions in the constructor of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Francesco should probably give final approval for this, but it all looks good to me. I left comments in response to the questions raised.
@pannarale this is now ready for your review. |
Calculating the coherent network SNR (
pycbc_multi_inspiral
) requires a grid of points in the sky. This adds a module and class to manage such grids in a more centralized way.Standard information about the request
This is mostly refactoring existing code into a new module. This change affects PyGRB but does not change any output.
Motivation
pycbc_multi_inspiral
is a very long script at this point, andpycbc_make_sky_grid
might also become very long as we implement various algorithms for placing sky grids available in the literature. Centralizing the management of sky grids, including reading/writing them from/to files, seems beneficial.Contents
I added a pycbc.tmpltbank.sky_grid module which currently contains a
SkyGrid
class, with methods to read and write sky grids from HDF5 files, calculate the antenna patterns and time delays over the grid for a network of detectors, and handle CLI arguments related to sky grids.Links to any issues or associated PRs
None.
Testing performed
I wrote a short script to plot the antenna patterns and time delays over a sky grids of random points for various detectors, and visually checked that those made sense:
Additional notes
None.